-
Notifications
You must be signed in to change notification settings - Fork 547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check certificate policy flags with only a certificate #1869
Conversation
Flags such as cert-email and cert-oidc-issuer were only checked when a certificate and its chain were passed to Cosign, or when the certificate is fetched from either the OCI annotation or from Rekor (for verify-blob). This adds support for checking certificate policy flags when only a certificate is passed to Cosign. Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
cc @znewman01 |
Note that I can't reuse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor in verify.go
!
Verified that this fixes my original problem:
$ git checkout hayden/support-cert-checks
$ make
$ ./cosign verify-blob /dev/null --cert /tmp/null.crt --signature /tmp/null.sig --cert-email bad@example.com
Error: verifying blob [/dev/null]: expected email not found in certificate
main.go:52: error during command execution: verifying blob [/dev/null]: expected email not found in certificate
@@ -212,7 +241,7 @@ func ValidateAndUnpackCert(cert *x509.Certificate, co *CheckOpts) (signature.Ver | |||
if identity.Subject != "" { | |||
regex, err := regexp.Compile(identity.Subject) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR but it's a regex??? That's a surprise
Does this open up an attack where someone passes zjn@mail.example.com
and I go register mailqexample.com
and can pass the check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vaikas - FYI, I think you added this originally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I sure did add it. @znewman01, in your particular example, yeah, you'd have to use:
zjn@mail\.example\.com
Let's take this off the thread however, and if we want this to be a non-regexp, I'd be happy to make the change. Would you mind creating an issue, tagging me in it so we have a proper track record and I'll go change it so that it is what folks want.
Thanks for the flagging!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK will do :)
Flags such as cert-email and cert-oidc-issuer were only checked when a
certificate and its chain were passed to Cosign, or when the certificate
is fetched from either the OCI annotation or from Rekor (for
verify-blob). This adds support for checking certificate policy flags
when only a certificate is passed to Cosign.
Signed-off-by: Hayden Blauzvern hblauzvern@google.com
Summary
Ticket Link
Fixes #1848
Release Note